Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CMAKE] Adding allias for pqxx target #784

Merged
merged 1 commit into from
Jan 19, 2024
Merged

[CMAKE] Adding allias for pqxx target #784

merged 1 commit into from
Jan 19, 2024

Conversation

alexv-ds
Copy link
Contributor

Why?

This will improve consistency between different ways of adding libpqxx to the project (specifically between find_package(libpqxx) and add_subdirectory(/subproject/path))

Example code that will be perfectly clean after this change

There is a minimalistic batch manager for cmake that is a thin wrapper over FetchContent, with some improvements - CPM.cmake. It has the ability to check for an installed version via find_package() before loading a package and the lack of consistency between find_package() and add_subriddectory() breaks the beauty of the code.

CPMAddPackage(
  NAME libpqxx
  GIT_TAG 7.8.1
  GITHUB_REPOSITORY jtv/libpqxx
)

target_link_libraries(${PROJECT_NAME} PRIVATE libpqxx::pqxx) # fail with add_subdirectory(), success with find_package()
target_link_libraries(${PROJECT_NAME} PRIVATE pqxx) # fail with find_package(), success with add_subdirectory()

# Note - in the second case the error will not be at configuration time, but at linking time.

If we add an alias to pqxx - the problem is solved. We will be able to use libpqxx::pqxx in both cases.

@jtv
Copy link
Owner

jtv commented Jan 19, 2024

Thank you! @alexv-ds I did see a note from @tt4g in my email, but I don't see it here. I guess that has already been addressed?

@jtv
Copy link
Owner

jtv commented Jan 19, 2024

Thank you! @alexv-ds I did see a note from @tt4g in my email, but I don't see it here. I guess that has already been addressed?

Oh never mind, I think that was an entirely different PR that's already been retracted!

@jtv jtv merged commit 6a9e330 into jtv:master Jan 19, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants